On Tue, Aug 26, 2014 at 03:35:36PM -0400, Ashwin Chaugule wrote:
> The PCC (Platform Communication Channel) is a generic
> mailbox to be used by PCC clients such as CPPC, RAS
> and MPST as defined in the ACPI 5.0+ spec. This patch
> modifies the Mailbox framework to lookup PCC mailbox
> controllers and channels such that PCC drivers can work
> with or without ACPI support in the kernel.
>
> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@xxxxxxxxxx>
> ---
> drivers/mailbox/mailbox.c | 118 ++++++++++++++++++++++++++++++++++---
> include/linux/mailbox_client.h | 6 ++
> include/linux/mailbox_controller.h | 1 +
> 3 files changed, 118 insertions(+), 7 deletions(-)
Based on the patch description (adding support for a particular kind of
mailbox) I'd expect to see a new driver or helper library being added to
drivers/mailbox rather than changes in the mailbox core. If changes in
the core are needed to support this I'd expect to see them broken out as
separate patches.
> +static struct mbox_controller *
> +mbox_find_pcc_controller(char *name)
> +{
> + struct mbox_controller *mbox;
> + list_for_each_entry(mbox, &mbox_cons, node) {
> + if (mbox->name)
> + if (!strcmp(mbox->name, name))
> + return mbox;
> + }
> +
> + return NULL;
> +}
This doesn't look particularly PCC specific?
> /* Sanity check */
> - if (!mbox || !mbox->dev || !mbox->ops || !mbox->num_chans)
> +
> + /*
> + * PCC clients and controllers are currently not backed by
> + * platform device structures.
> + */
> +#ifndef CONFIG_PCC
> + if (!mbox->dev)
> + return -EINVAL;
> +#endif
It seems better to make this consistent - either enforce it all the time
or don't enforce it.
Attachment:
signature.asc
Description: Digital signature
