Re: [External] : [PATCH v30 2/3] mailbox: pcc: functions for reading and writing PCC extended data

ALOK TIWARI
2025-10-17T10:12:43Z

On 10/17/2025 2:32 AM, Adam Young wrote:
> Adds functions that aid in compliance with the PCC protocol by
> checking the command complete flag status.
>
> Adds a function that exposes the size of the shared buffer without
> activating the channel.
>
> Adds a function that allows a client to query the number of bytes
> avaialbel to read in order to preallocate buffers for reading.

/s available

>
> Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
> ---
> drivers/mailbox/pcc.c | 129 ++++++++++++++++++++++++++++++++++++++++++
> include/acpi/pcc.h | 38 +++++++++++++
> 2 files changed, 167 insertions(+)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 978a7b674946..653897d61db5 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -367,6 +367,46 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
> return IRQ_HANDLED;
> }
>
> +static
> +struct pcc_chan_info *lookup_channel_info(int subspace_id)
> +{
> + struct pcc_chan_info *pchan;
> + struct mbox_chan *chan;
> +
> + if (subspace_id < 0 || subspace_id >= pcc_chan_count)
> + return ERR_PTR(-ENOENT);
> +
> + pchan = chan_info + subspace_id;
> + chan = pchan->chan.mchan;
> + if (IS_ERR(chan) || chan->cl) {
> + pr_err("Channel not found for idx: %d\n", subspace_id);
> + return ERR_PTR(-EBUSY);
> + }
> + return pchan;
> +}
> +
> +/**
> + * pcc_mbox_buffer_size - PCC clients call this function to
> + * request the size of the shared buffer in cases
> + * where requesting the channel would prematurely
> + * trigger channel activation and message delivery.
> + * @subspace_id: The PCC Subspace index as parsed in the PCC client
> + * ACPI package. This is used to lookup the array of PCC
> + * subspaces as parsed by the PCC Mailbox controller.
> + *
> + * Return: The size of the shared buffer.
> + */
> +int pcc_mbox_buffer_size(int index)

use subspace_id for consistent name and use in header

> +{
> + struct pcc_chan_info *pchan = lookup_channel_info(index);
> +
> + if (IS_ERR(pchan))
> + return -1;
> + return pchan->chan.shmem_size;
> +}
> +EXPORT_SYMBOL_GPL(pcc_mbox_buffer_size);
> +
> +
> /**
> * pcc_mbox_request_channel - PCC clients call this function to
> * request a pointer to their PCC subspace, from which they
> @@ -437,6 +477,95 @@ void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
> }
> EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
>
> +/**
> + * pcc_mbox_query_bytes_available
> + *
> + * @pchan pointer to channel associated with buffer
> + * Return: the number of bytes available to read from the shared buffer
> + */
> +int pcc_mbox_query_bytes_available(struct pcc_mbox_chan *pchan)
> +{
> + struct pcc_extended_header pcc_header;
> + struct pcc_chan_info *pinfo = pchan->mchan->con_priv;
> + int data_len;
> + u64 val;
> +
> + pcc_chan_reg_read(&pinfo->cmd_complete, &val);
> + if (val) {
> + pr_info("%s Buffer not enabled for reading", __func__);
> + return -1;
> + }
> + memcpy_fromio(&pcc_header, pchan->shmem,
> + sizeof(pcc_header));
> + data_len = pcc_header.length - sizeof(u32) + sizeof(pcc_header);
> + return data_len;
> +}
> +EXPORT_SYMBOL_GPL(pcc_mbox_query_bytes_available);
> +
> +/**
> + * pcc_mbox_read_from_buffer - Copy bytes from shared buffer into data
> + *
> + * @pchan - channel associated with the shared buffer
> + * @len - number of bytes to read
> + * @data - pointer to memory in which to write the data from the
> + * shared buffer
> + *
> + * Return: number of bytes read and written into daa

typo daa -> data

> + */
> +int pcc_mbox_read_from_buffer(struct pcc_mbox_chan *pchan, int len, void *data)
> +{
> + struct pcc_chan_info *pinfo = pchan->mchan->con_priv;
> + int data_len;
> + u64 val;
> +
> + pcc_chan_reg_read(&pinfo->cmd_complete, &val);
> + if (val) {
> + pr_info("%s buffer not enabled for reading", __func__);
> + return -1;
> + }
> + data_len = pcc_mbox_query_bytes_available(pchan);
> + if (len < data_len)
> + data_len = len;
> + memcpy_fromio(data, pchan->shmem, len);
> + return len;
> +}
> +EXPORT_SYMBOL_GPL(pcc_mbox_read_from_buffer);
> +
> +/**
> + * pcc_mbox_write_to_buffer, copy the contents of the data
> + * pointer to the shared buffer. Confirms that the command
> + * flag has been set prior to writing. Data should be a
> + * properly formatted extended data buffer.
> + * pcc_mbox_write_to_buffer
> + * @pchan: channel
> + * @len: Length of the overall buffer passed in, including the
> + * Entire header. The length value in the shared buffer header
> + * Will be calculated from len.
> + * @data: Client specific data to be written to the shared buffer.
> + * Return: number of bytes written to the buffer.
> + */
> +int pcc_mbox_write_to_buffer(struct pcc_mbox_chan *pchan, int len, void *data)
> +{
> + struct pcc_extended_header *pcc_header = data;
> + struct mbox_chan *mbox_chan = pchan->mchan;
> +
> + /*
> + * The PCC header length includes the command field
> + * but not the other values from the header.
> + */
> + pcc_header->length = len - sizeof(struct pcc_extended_header) + sizeof(u32);
> +
> + if (!pcc_last_tx_done(mbox_chan)) {
> + pr_info("%s pchan->cmd_complete not set.", __func__);
> + return 0;
> + }
> + memcpy_toio(pchan->shmem, data, len);
> +
> + return len;
> +}
> +EXPORT_SYMBOL_GPL(pcc_mbox_write_to_buffer);
> +
> +
> /**
> * pcc_send_data - Called from Mailbox Controller code. Used
> * here only to ring the channel doorbell. The PCC client
> diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
> index 840bfc95bae3..96a6f85fc1ba 100644
> --- a/include/acpi/pcc.h
> +++ b/include/acpi/pcc.h
> @@ -19,6 +19,13 @@ struct pcc_mbox_chan {
> u16 min_turnaround_time;
> };
>
> +struct pcc_extended_header {
> + u32 signature;
> + u32 flags;
> + u32 length;
> + u32 command;
> +};
> +
> /* Generic Communications Channel Shared Memory Region */
> #define PCC_SIGNATURE 0x50434300
> /* Generic Communications Channel Command Field */
> @@ -37,6 +44,17 @@ struct pcc_mbox_chan {
> extern struct pcc_mbox_chan *
> pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
> extern void pcc_mbox_free_channel(struct pcc_mbox_chan *chan);
> +extern
> +int pcc_mbox_write_to_buffer(struct pcc_mbox_chan *pchan, int len, void *data);
> +extern
> +int pcc_mbox_query_bytes_available(struct pcc_mbox_chan *pchan);
> +extern
> +int pcc_mbox_read_from_buffer(struct pcc_mbox_chan *pchan, int len,
> + void *data);
> +extern
> +int pcc_mbox_buffer_size(int index);
> +
> +
> #else
> static inline struct pcc_mbox_chan *
> pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
> @@ -44,6 +62,26 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
> return ERR_PTR(-ENODEV);
> }
> static inline void pcc_mbox_free_channel(struct pcc_mbox_chan *chan) { }
> +static inline
> +int pcc_mbox_write_to_buffer(struct pcc_mbox_chan *pchan, int len, void *data)
> +{
> + return 0;
> +}
> +static inline int pcc_mbox_query_bytes_available(struct pcc_mbox_chan *pchan);

remove ;

> +{
> + return 0;
> +}
> +static inline
> +int pcc_mbox_read_from_buffer(struct pcc_mbox_chan *pchan, int len, void *data)
> +{
> + return 0;
> +}
> +static inline
> +int pcc_mbox_buffer_size(int index)
> +{
> + return -1;
> +}
> +
> #endif
>
> #endif /* _PCC_H */

Thanks,
Alok