Re: [PATCH RFC 09/13] dmaengine: sdxi: Add core device management code

Nathan Lynch
2025-09-16T21:23:39Z

Jonathan Cameron <jonathan.cameron@huawei.com> writes:
> On Fri, 05 Sep 2025 13:48:32 -0500
> Nathan Lynch via B4 Relay <devnull+nathan.lynch.amd.com@kernel.org> wrote:
>
>> From: Nathan Lynch <nathan.lynch@amd.com>
>>
>> Add code that manages device initialization and exit and provides
>> entry points for the PCI driver code to come.
>
> I'd prefer a patch series that started with the PCI device and built up
> functionality for the stuff found earlier + in this patch on top of it.
> Doing that allows each patch to be fully tested and reviewed on it's
> own.

Right, I think you made this point elsewhere too -- I'll try organizing
the series as you suggest.

>> +/* Refer to "Activation of the SDXI Function by Software". */
>> +static int sdxi_fn_activate(struct sdxi_dev *sdxi)
>> +{
>> + const struct sdxi_dev_ops *ops = sdxi->dev_ops;
>> + u64 cxt_l2;
>> + u64 cap0;
>> + u64 cap1;
>> + u64 ctl2;
>
> Combine these u64 declarations on one line.

Sure.

>> +void sdxi_device_exit(struct sdxi_dev *sdxi)
>> +{
>> + sdxi_working_cxt_exit(sdxi->dma_cxt);
>> +
>> + /* Walk sdxi->cxt_array freeing any allocated rows. */
>> + for (size_t i = 0; i < L2_TABLE_ENTRIES; ++i) {
>> + if (!sdxi->cxt_array[i])
>> + continue;
>> + /* When a context is released its entry in the table should be NULL. */
>> + for (size_t j = 0; j < L1_TABLE_ENTRIES; ++j) {
>> + struct sdxi_cxt *cxt = sdxi->cxt_array[i][j];
>> +
>> + if (!cxt)
>> + continue;
>> + if (cxt->id != 0) /* admin context shutdown is last */
>> + sdxi_working_cxt_exit(cxt);
>> + sdxi->cxt_array[i][j] = NULL;
>> + }
>> + if (i != 0) /* another special case for admin cxt */
>> + kfree(sdxi->cxt_array[i]);
>> + }
>> +
>> + sdxi_working_cxt_exit(sdxi->admin_cxt);
>> + kfree(sdxi->cxt_array[0]); /* ugh */
>
> The constraints here need to be described a little more clearly.

Heh, I think I was tired of finding and fixing memory leaks when I wrote
these comments. If this part of the code survives to the next version
I'll improve them, but I plan to rework how we track contexts using a
per-device xarray, so this code should become a single xa_for_each()
loop followed by the admin context cleanup.