diff mbox

[v9] Mailbox: Add support for Platform Communication Channel

Message ID CAJ5Y-eY5XtViMjU4FC-q1vCSgFa69DLGFeRr2GTmEO0CtnZPtA@mail.gmail.com
State New
Headers show

Commit Message

Ashwin Chaugule Oct. 23, 2014, 3:36 p.m. UTC
Hello,

On 10 October 2014 16:13, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
> ACPI 5.0+ spec defines a generic mode of communication
> between the OS and a platform such as the BMC. This medium
> (PCC) is typically used by CPPC (ACPI CPU Performance management),
> RAS (ACPI reliability protocol) and MPST (ACPI Memory power
> states).
>
> This patch adds PCC support as a Mailbox Controller.
>
> Reviewed-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
> ---
>  drivers/mailbox/Kconfig   |  12 ++
>  drivers/mailbox/Makefile  |   2 +
>  drivers/mailbox/mailbox.c |   4 +-
>  drivers/mailbox/mailbox.h |  16 ++
>  drivers/mailbox/pcc.c     | 369 ++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 400 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/mailbox/mailbox.h
>  create mode 100644 drivers/mailbox/pcc.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index 9fd9c67..c04fed9 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -33,4 +33,16 @@ config OMAP_MBOX_KFIFO_SIZE
>           Specify the default size of mailbox's kfifo buffers (bytes).
>           This can also be changed at runtime (via the mbox_kfifo_size
>           module parameter).
> +
> +config PCC
> +       bool "Platform Communication Channel Driver"
> +       depends on ACPI
> +       help
> +         ACPI 5.0+ spec defines a generic mode of communication
> +         between the OS and a platform such as the BMC. This medium
> +         (PCC) is typically used by CPPC (ACPI CPU Performance management),
> +         RAS (ACPI reliability protocol) and MPST (ACPI Memory power
> +         states). Select this driver if your platform implements the
> +         PCC clients mentioned above.
> +
>  endif

Looks like Linus has agreed to pull in the Mailbox framework in 3.18
[1], so I was hoping we could move this patch forward.

When I started working on the PCC driver, I based it off of ACPI v5.0.
Since then the spec moved to v5.1 and added a trivial change to the
PCC section. This change adds a new subtable to support systems that
dont have an SCI for the doorbell irq. The new structure declaration
is already in mainline [2]. Given that there were no existing users of
PCC so far and that the future users are more likely to use the new
subtable, I'd like to include it here and make it the default from the
start. SCI support can be added later if it is needed. The change to
do this is also trivial and the diff is below.

Mark, are you okay with squashing this diff into the original patch
[v9] and keeping your reviewed-by tag? I can then send a git pull
request.

Arnd, Rafael, if there are no further comments on this patch, kindly
advise which tree to rebase on for the git pull.

--------------------8<-----------------------

     doorbell = pcct_ss->doorbell_register;
@@ -219,8 +217,7 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
     acpi_write((doorbell_val & doorbell_preserve) | doorbell_write,
             &doorbell);

-out_err:
-    return ret;
+    return 0;
 }

 static struct mbox_chan_ops pcc_chan_ops = {
@@ -241,12 +238,12 @@ static struct mbox_chan_ops pcc_chan_ops = {
 static int parse_pcc_subspace(struct acpi_subtable_header *header,
         const unsigned long end)
 {
-    struct acpi_pcct_subspace *pcct_ss;
+    struct acpi_pcct_hw_reduced *pcct_ss;

     if (pcc_mbox_ctrl.num_chans <= MAX_PCC_SUBSPACES) {
-        pcct_ss = (struct acpi_pcct_subspace *) header;
+        pcct_ss = (struct acpi_pcct_hw_reduced *) header;

-        if (pcct_ss->header.type != ACPI_PCCT_TYPE_GENERIC_SUBSPACE) {
+        if (pcct_ss->header.type != ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE) {
             pr_err("Incorrect PCC Subspace type detected\n");
             return -EINVAL;
         }
@@ -287,7 +284,7 @@ static int __init acpi_pcc_probe(void)

     count = acpi_table_parse_entries(ACPI_SIG_PCCT,
             sizeof(struct acpi_table_pcct),
-            ACPI_PCCT_TYPE_GENERIC_SUBSPACE,
+            ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE,
             parse_pcc_subspace, MAX_PCC_SUBSPACES);

     if (count <= 0) {

---------------8------------------------


Thanks,
Ashwin

[1] - https://lkml.org/lkml/2014/10/21/660
[2] - https://lkml.org/lkml/2014/7/30/9

Comments

Ashwin Chaugule Nov. 6, 2014, 12:48 a.m. UTC | #1
Hi,

On 23 October 2014 11:36, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
> Hello,
>
> On 10 October 2014 16:13, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
>> ACPI 5.0+ spec defines a generic mode of communication
>> between the OS and a platform such as the BMC. This medium
>> (PCC) is typically used by CPPC (ACPI CPU Performance management),
>> RAS (ACPI reliability protocol) and MPST (ACPI Memory power
>> states).
>>
>> This patch adds PCC support as a Mailbox Controller.
>>
>> Reviewed-by: Mark Brown <broonie@kernel.org>
>> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
>> ---
>>  drivers/mailbox/Kconfig   |  12 ++
>>  drivers/mailbox/Makefile  |   2 +
>>  drivers/mailbox/mailbox.c |   4 +-
>>  drivers/mailbox/mailbox.h |  16 ++
>>  drivers/mailbox/pcc.c     | 369 ++++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 400 insertions(+), 3 deletions(-)
>>  create mode 100644 drivers/mailbox/mailbox.h
>>  create mode 100644 drivers/mailbox/pcc.c
>>
>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
>> index 9fd9c67..c04fed9 100644
>> --- a/drivers/mailbox/Kconfig
>> +++ b/drivers/mailbox/Kconfig
>> @@ -33,4 +33,16 @@ config OMAP_MBOX_KFIFO_SIZE
>>           Specify the default size of mailbox's kfifo buffers (bytes).
>>           This can also be changed at runtime (via the mbox_kfifo_size
>>           module parameter).
>> +
>> +config PCC
>> +       bool "Platform Communication Channel Driver"
>> +       depends on ACPI
>> +       help
>> +         ACPI 5.0+ spec defines a generic mode of communication
>> +         between the OS and a platform such as the BMC. This medium
>> +         (PCC) is typically used by CPPC (ACPI CPU Performance management),
>> +         RAS (ACPI reliability protocol) and MPST (ACPI Memory power
>> +         states). Select this driver if your platform implements the
>> +         PCC clients mentioned above.
>> +
>>  endif
>
> Looks like Linus has agreed to pull in the Mailbox framework in 3.18
> [1], so I was hoping we could move this patch forward.
>
> When I started working on the PCC driver, I based it off of ACPI v5.0.
> Since then the spec moved to v5.1 and added a trivial change to the
> PCC section. This change adds a new subtable to support systems that
> dont have an SCI for the doorbell irq. The new structure declaration
> is already in mainline [2]. Given that there were no existing users of
> PCC so far and that the future users are more likely to use the new
> subtable, I'd like to include it here and make it the default from the
> start. SCI support can be added later if it is needed. The change to
> do this is also trivial and the diff is below.
>
> Mark, are you okay with squashing this diff into the original patch
> [v9] and keeping your reviewed-by tag? I can then send a git pull
> request.
>
> Arnd, Rafael, if there are no further comments on this patch, kindly
> advise which tree to rebase on for the git pull.

Guessing there are no objections to this patch and the diff? Can we
please merge this upstream?

Thanks,
Ashwin

>
> --------------------8<-----------------------
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 0006fc3..592c6fa 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -130,7 +130,7 @@ EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
>   */
>  static bool pcc_tx_done(struct mbox_chan *chan)
>  {
> -    struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
> +    struct acpi_pcct_hw_reduced *pcct_ss = chan->con_priv;
>      struct acpi_pcct_shared_memory *generic_comm_base =
>          (struct acpi_pcct_shared_memory *) pcct_ss->base_address;
>      u16 cmd_delay = pcct_ss->min_turnaround_time;
> @@ -182,7 +182,7 @@ static int get_subspace_id(struct mbox_chan *chan)
>   */
>  static int pcc_send_data(struct mbox_chan *chan, void *data)
>  {
> -    struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
> +    struct acpi_pcct_hw_reduced *pcct_ss = chan->con_priv;
>      struct acpi_pcct_shared_memory *generic_comm_base =
>          (struct acpi_pcct_shared_memory *) pcct_ss->base_address;
>      struct acpi_generic_address doorbell;
> @@ -191,14 +191,12 @@ static int pcc_send_data(struct mbox_chan *chan,
> void *data)
>      u64 doorbell_write;
>      u16 cmd = *(u16 *) data;
>      u16 ss_idx = -1;
> -    int ret = 0;
>
>      ss_idx = get_subspace_id(chan);
>
>      if (ss_idx < 0) {
>          pr_err("Invalid Subspace ID from PCC client\n");
> -        ret = -EINVAL;
> -        goto out_err;
> +        return -EINVAL;
>      }
>
>      doorbell = pcct_ss->doorbell_register;
> @@ -219,8 +217,7 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
>      acpi_write((doorbell_val & doorbell_preserve) | doorbell_write,
>              &doorbell);
>
> -out_err:
> -    return ret;
> +    return 0;
>  }
>
>  static struct mbox_chan_ops pcc_chan_ops = {
> @@ -241,12 +238,12 @@ static struct mbox_chan_ops pcc_chan_ops = {
>  static int parse_pcc_subspace(struct acpi_subtable_header *header,
>          const unsigned long end)
>  {
> -    struct acpi_pcct_subspace *pcct_ss;
> +    struct acpi_pcct_hw_reduced *pcct_ss;
>
>      if (pcc_mbox_ctrl.num_chans <= MAX_PCC_SUBSPACES) {
> -        pcct_ss = (struct acpi_pcct_subspace *) header;
> +        pcct_ss = (struct acpi_pcct_hw_reduced *) header;
>
> -        if (pcct_ss->header.type != ACPI_PCCT_TYPE_GENERIC_SUBSPACE) {
> +        if (pcct_ss->header.type != ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE) {
>              pr_err("Incorrect PCC Subspace type detected\n");
>              return -EINVAL;
>          }
> @@ -287,7 +284,7 @@ static int __init acpi_pcc_probe(void)
>
>      count = acpi_table_parse_entries(ACPI_SIG_PCCT,
>              sizeof(struct acpi_table_pcct),
> -            ACPI_PCCT_TYPE_GENERIC_SUBSPACE,
> +            ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE,
>              parse_pcc_subspace, MAX_PCC_SUBSPACES);
>
>      if (count <= 0) {
>
> ---------------8------------------------
>
>
> Thanks,
> Ashwin
>
> [1] - https://lkml.org/lkml/2014/10/21/660
> [2] - https://lkml.org/lkml/2014/7/30/9
diff mbox

Patch

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 0006fc3..592c6fa 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -130,7 +130,7 @@  EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
  */
 static bool pcc_tx_done(struct mbox_chan *chan)
 {
-    struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
+    struct acpi_pcct_hw_reduced *pcct_ss = chan->con_priv;
     struct acpi_pcct_shared_memory *generic_comm_base =
         (struct acpi_pcct_shared_memory *) pcct_ss->base_address;
     u16 cmd_delay = pcct_ss->min_turnaround_time;
@@ -182,7 +182,7 @@  static int get_subspace_id(struct mbox_chan *chan)
  */
 static int pcc_send_data(struct mbox_chan *chan, void *data)
 {
-    struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
+    struct acpi_pcct_hw_reduced *pcct_ss = chan->con_priv;
     struct acpi_pcct_shared_memory *generic_comm_base =
         (struct acpi_pcct_shared_memory *) pcct_ss->base_address;
     struct acpi_generic_address doorbell;
@@ -191,14 +191,12 @@  static int pcc_send_data(struct mbox_chan *chan,
void *data)
     u64 doorbell_write;
     u16 cmd = *(u16 *) data;
     u16 ss_idx = -1;
-    int ret = 0;

     ss_idx = get_subspace_id(chan);

     if (ss_idx < 0) {
         pr_err("Invalid Subspace ID from PCC client\n");
-        ret = -EINVAL;
-        goto out_err;
+        return -EINVAL;
     }