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

[Nathan Lynch]

Jonathan Cameron <jonathan.cameron@xxxxxxxxxx> writes:
> On Fri, 05 Sep 2025 13:48:32 -0500
> Nathan Lynch via B4 Relay <devnull+nathan.lynch.amd.com@xxxxxxxxxx> wrote:
>
>> From: Nathan Lynch <nathan.lynch@xxxxxxx>
>> 
>> 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.