Hello,

On 23 June 2014 15:10, Arnd Bergmann [off-list ref] wrote:

quoted

Fair point. The more I think about this, it seems that if we want to
use the mailbox framework for ACPI kernels, we should have a PCC
specific bypass, something like the one you suggested below. The ACPI
spec defines PCC as the only "mailbox" like mechanism. There are 3 PCC
clients defined as well; CPPC, MPST and RASF. Each of these have their
own ACPI tables and so they dont require special DSDT entries.

Ok, I see. Can you describe what data is in these tables?

For CPPC, its a field for version number, number of entries and then
followed by a bunch of PCC entries that have the following structure:

 51 struct pcc_register_resource {
 52     u8 descriptor;
 53     u16 length;
 54     u8 space_id;
 55     u8 bit_width;
 56     u8 bit_offset;
 57     u8 access_size;
 58     u64 address;
 59 } __attribute__ ((packed));

These essentially describe the PCC register space to be used by the
respective protocol. e.g. CPPC uses these to exchange CPU performance
metrics between the OS and the firmware.
I believe MPST and RASF also follow the same format.

quoted

Moreover, these PCC client drivers will be very ACPI specific anyway.
So, trying to emulate DT like mbox controller-client matching in ACPI
at this point is rather pointless. It will require creating dummy DSDT
entries for the PCC mailbox controller and PCC mailbox clients which
have their own well defined ACPI tables (and so dont belong in the OS
agnostic DSDT) and then coming up with customized Device Specific
Methods (DSMs) for mbox clients to refer to mbox controllers.

Actually you wouldn't necessarily need DSDT entries, the ACPI core could
just call platform_device_create() to instantiate the devices based on
the PCC tables.

quoted

The other alternative is to skip the mailbox framework altogether. One
thing to note is that the PCC driver and its clients should work on
X86, ARMv8 and any other platform that has ACPI support. Currently the
Mailbox framework looks platform agnostic but is tied to DT, so it may
not work well for everyone. But like I mentioned early on, the
framework provides for async notification and queuing which is useful
for PCC, so I'd prefer the PCC specific bypass option.

The mailbox API should still work fine without DT, it would be easy
enough to add a lookup mechanism for architectures that create their
own platform devices from hardcoded kernel structures, or from ACPI
tables that are meant to emulate the DT bindings on embedded x86.

Right, a generic lookup method would be useful. I think we should
probably revisit this option when/if there are ACPI cases which use
anything other than the PCC mailbox controller.

But treating PCC special probably does make most sense here, at
least the lookup path.

Agreed.

quoted

quoted

The alternative would be not to use mbox_request_channel() at all
for now, but to add a new interface that can only be used PCC and
that matches by ID but is independent of the use of ACPI or DT,
something like:

struct mbox_chan *pcc_mbox_get_channel(struct mbox_client *cl,
                        char *name, unsigned chan_id,
                        struct mbox_chan **chan)
{
        struct mbox_controller *mbox;
        mbox = mbox_find_pcc_controller(name, ...);

        *chan = &mbox->chans[chan_id];
        return init_channel(*chan, cl);
}

This would mean that we'd have to special-case "pcc" users, which is
not very nice, but at least it would work on both DT and ACPI,
and a future ACPI version could still add support for the mailbox
API later.

I'll play around with this idea a bit and see how it looks.

Ok, thanks for looking into this.

Cheers,
Ashwin