Re: [PATCH 05/13] mailbox: pcc: Consolidate subspace doorbell register parsing

From: Cristian Marussi
Date: Wed Jul 14 2021 - 14:09:46 EST


On Thu, Jul 08, 2021 at 07:08:43PM +0100, Sudeep Holla wrote:
> Extended PCC subspaces(Type 3 and 4) differs from generic(Type 0) and
nit: s/differs/differ

> HW-Reduced Communication(Type 1 and 2) subspace structures. However some
> fields share same offsets and same type of structure can be use to
> extract the fields. In order to simplify that, let us move all the doorbell
> register parsing into pcc_parse_subspace_db_reg and consolidate there.
> It will be easier to extend it if required within the same.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx>
> ---
> drivers/mailbox/pcc.c | 31 +++++++++++++++++++++++--------
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 55866676a508..5f19bee71c04 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -447,6 +447,28 @@ static int pcc_parse_subspace_irq(struct pcc_chan_info *pchan,
> return 0;
> }
>
> +/**
> + * pcc_parse_subspace_db_reg - Parse the PCC doorbell register
> + *
> + * @pchan: Pointer to the PCC channel info structure.
> + * @pcct_entry: Pointer to the ACPI subtable header.
> + *
> + */
> +static void pcc_parse_subspace_db_reg(struct pcc_chan_info *pchan,
> + struct acpi_subtable_header *pcct_entry)
> +{
> + struct acpi_pcct_subspace *pcct_ss;
> + struct acpi_generic_address *db_reg;
> +
> + pcct_ss = (struct acpi_pcct_subspace *)pcct_entry;
> +
> + /* If doorbell is in system memory cache the virt address */
> + db_reg = &pcct_ss->doorbell_register;
> + if (db_reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
> + pchan->db_vaddr = acpi_os_ioremap(db_reg->address,
> + db_reg->bit_width / 8);
> +}
> +
> /**
> * acpi_pcc_probe - Parse the ACPI tree for the PCCT.
> *
> @@ -512,8 +534,6 @@ static int __init acpi_pcc_probe(void)
>
> for (i = 0; i < count; i++) {
> struct pcc_chan_info *pchan = chan_info + i;
> - struct acpi_generic_address *db_reg;
> - struct acpi_pcct_subspace *pcct_ss;
> pcc_mbox_channels[i].con_priv = pcct_entry;
>
> if (pcc_mbox_ctrl.txdone_irq) {
> @@ -521,13 +541,8 @@ static int __init acpi_pcc_probe(void)
> if (rc < 0)
> goto err;
> }
> - pcct_ss = (struct acpi_pcct_subspace *) pcct_entry;
> + pcc_parse_subspace_db_reg(pchan, pcct_entry);
>
> - /* If doorbell is in system memory cache the virt address */
> - db_reg = &pcct_ss->doorbell_register;
> - if (db_reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
> - pchan->db_vaddr = acpi_os_ioremap(db_reg->address,
> - db_reg->bit_width / 8);
> pcct_entry = (struct acpi_subtable_header *)
> ((unsigned long) pcct_entry + pcct_entry->length);
> }
> --
> 2.25.1
>

Reviewed-by: Cristian Marussi <cristian.marussi@xxxxxxx>

Thanks,
Cristian