Re: [patch 02/37] device: Add device::msi_data pointer and struct msi_device_data

[Thomas Gleixner]

On Sat, Nov 27 2021 at 20:14, Jason Gunthorpe wrote:
> On Sat, Nov 27, 2021 at 02:20:09AM +0100, Thomas Gleixner wrote:
>
>> +/**
>> + * msi_setup_device_data - Setup MSI device data
>> + * @dev:	Device for which MSI device data should be set up
>> + *
>> + * Return: 0 on success, appropriate error code otherwise
>> + *
>> + * This can be called more than once for @dev. If the MSI device data is
>> + * already allocated the call succeeds. The allocated memory is
>> + * automatically released when the device is destroyed.
>
> I would say 'by devres when the driver is removed' rather than device
> is destroyed - to me the latter implies it would happen at device_del
> or perhaps during release..

Ah. Indeed it's when the driver unbinds because that's what disables MSI.

>> +int msi_setup_device_data(struct device *dev)
>> +{
>> +	struct msi_device_data *md;
>> +
>> +	if (dev->msi.data)
>> +		return 0;
>> +
>> +	md = devres_alloc(msi_device_data_release, sizeof(*md), GFP_KERNEL);
>> +	if (!md)
>> +		return -ENOMEM;
>> +
>> +	raw_spin_lock_init(&md->lock);
>
> I also couldn't guess why this needed to be raw?

That lock is to replace the raw spinlock we have in struct device right
now, which is protecting low level register access and that's called
from within irq_desc::lock held regions with interrupts disabled. I had
to stick something into the data structure because allocating 0 bytes is
invalid. But yes, I should have mentioned it in the changelog.

Thanks,

        tglx