Re: [PATCH v5 1/3] mctp pcc: Check before sending MCTP PCC response ACK
From: Jonathan Cameron
Date: Mon Sep 16 2024 - 05:52:13 EST
On Fri, 13 Sep 2024 17:21:06 -0400
Adam Young <admiyo@xxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> >>+ * @shmem_base_addr: the virtual memory address of the shared buffer
>
> >If you are only going to map this from this pointer for the
> >initiator/responder shared memory region, maybe it would benefit
> >from a more specific name?
>
>
> I am not certain what would be more correct.
>
>
> On 8/1/24 07:41, Jonathan Cameron wrote:
>
> >> + pchan->shmem_base_addr = devm_ioremap(chan->mbox->dev,
> >> + pchan->chan.shmem_base_addr,
> >> + pchan->chan.shmem_size);
> > devm doesn't seem appropriate here given we have manual management
> > of other resources, so the ordering will be different in remove
> > vs probe.
> >
> > So I'd handle release of this manually in mbox_free_channel()
>
>
> How fixed are you on this? mbox_free_channel is the parent code, and
> knows nothing about this resource. It does no specific resource cleanup.
I've lost context on this unfortunately and don't have time to look
back at it this week. Maybe right answer is a cleanup callback?
>
> The only place we could release it is in the pcc_mbox_free, but that is
> essentially a call to the parent function.
>
> All other comments should be addressed in the next version.
>
>