Re: [PATCH 07/17] i3c: mipi-i3c-hci: Manage DMA deallocation via devres action

Frank Li
2025-12-19T16:32:57Z

On Fri, Dec 19, 2025 at 04:45:24PM +0200, Adrian Hunter wrote:
> The driver already uses devres for resource management, but the standard
> resource-managed DMA allocation helpers cannot be used because they assume
> the DMA device matches the managed device.
>
> To address this, factor out the deallocation logic from hci_dma_cleanup()
> into a new helper, hci_dma_free(), and register it as a devres action.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/i3c/master/mipi-i3c-hci/dma.c | 53 ++++++++++++++++++---------
> 1 file changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 54849aa98fad..703d5cf79d5e 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -146,30 +146,18 @@ struct hci_dma_dev_ibi_data {
> unsigned int max_len;
> };
>
> -static void hci_dma_cleanup(struct i3c_hci *hci)
> +static void hci_dma_free(void *data)

can you move hci_dma_free() below hci_dma_cleanup() to make patch look
better to review?

> {
> + struct i3c_hci *hci = data;
> struct hci_rings_data *rings = hci->io_data;
> struct hci_rh_data *rh;
> - unsigned int i;
>
> if (!rings)
> return;
>
> - for (i = 0; i < rings->total; i++) {
> - rh = &rings->headers[i];
> -
> - rh_reg_write(INTR_SIGNAL_ENABLE, 0);
> - rh_reg_write(RING_CONTROL, 0);
> - }
> -
> - i3c_hci_sync_irq_inactive(hci);
> -
> - for (i = 0; i < rings->total; i++) {
> + for (int i = 0; i < rings->total; i++) {
> rh = &rings->headers[i];
>
> - rh_reg_write(CR_SETUP, 0);
> - rh_reg_write(IBI_SETUP, 0);
> -
> if (rh->xfer)
> dma_free_coherent(rings->sysdev,
> rh->xfer_struct_sz * rh->xfer_entries,
> @@ -190,12 +178,38 @@ static void hci_dma_cleanup(struct i3c_hci *hci)
> kfree(rh->ibi_data);
> }
>
> - rhs_reg_write(CONTROL, 0);
> -
> kfree(rings);
> hci->io_data = NULL;
> }
>
> +static void hci_dma_cleanup(struct i3c_hci *hci)
> +{
> + struct hci_rings_data *rings = hci->io_data;
> + struct hci_rh_data *rh;
> + unsigned int i;
> +
> + if (!rings)
> + return;
> +
> + for (i = 0; i < rings->total; i++) {
> + rh = &rings->headers[i];
> +
> + rh_reg_write(INTR_SIGNAL_ENABLE, 0);
> + rh_reg_write(RING_CONTROL, 0);
> + }
> +
> + i3c_hci_sync_irq_inactive(hci);
> +
> + for (i = 0; i < rings->total; i++) {
> + rh = &rings->headers[i];
> +
> + rh_reg_write(CR_SETUP, 0);
> + rh_reg_write(IBI_SETUP, 0);
> + }
> +
> + rhs_reg_write(CONTROL, 0);
> +}
> +
> static int hci_dma_init(struct i3c_hci *hci)
> {
> struct hci_rings_data *rings;
> @@ -359,10 +373,15 @@ static int hci_dma_init(struct i3c_hci *hci)
> RING_CTRL_RUN_STOP);
> }
>
> + ret = devm_add_action(hci->master.dev.parent, hci_dma_free, hci);
> + if (ret)
> + goto err_out;
> +

use devm_add_action_or_reset(), so needn't goto

Frank
> return 0;
>
> err_out:
> hci_dma_cleanup(hci);
> + hci_dma_free(hci);
> return ret;
> }
>
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c