Luca Coelho <luca@xxxxxxxxx> writes:
> On Tue, 2018-04-24 at 12:44 +0300, Kalle Valo wrote:
>> Luca Coelho <luca@xxxxxxxxx> writes:
>>
>> > From: Luca Coelho <luciano.coelho@xxxxxxxxx>
>> >
>> > If we fail to to grab NIC access because the device is not
>> > responding
>> > (i.e. CSR_GP_CNTRL returns 0xFFFFFFFF), remove the device from the
>> > PCI
>> > bus, to avoid any further damage, and to let the user space rescan.
>> >
>> > Signed-off-by: Luca Coelho <luciano.coelho@xxxxxxxxx>
>> > Signed-off-by: Rajat Jain <rajatja@xxxxxxxxxx>
>>
>> The commit log doesn't mention anything about the module parameter
>> nor
>> about the kobject uevent.
>
> Good point. The uevent and the module parameter were added in later
> patches and I squashed them all into this one, but forgot to update the
> commit message. I'll fix it.
Good, thanks.
>> > +static void iwl_trans_pcie_removal_wk(struct work_struct *wk)
>> > +{
>> > + struct iwl_trans_pcie_removal *removal =
>> > + container_of(wk, struct iwl_trans_pcie_removal,
>> > work);
>> > + struct pci_dev *pdev = removal->pdev;
>> > + char *prop[] = {"EVENT=INACCESSIBLE", NULL};
>> > +
>> > + dev_err(&pdev->dev, "Device gone - attempting removal\n");
>> > + kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, prop);
>> > + pci_lock_rescan_remove();
>> > + pci_dev_put(pdev);
>> > + pci_stop_and_remove_bus_device(pdev);
>> > + pci_unlock_rescan_remove();
>> > +
>> > + kfree(removal);
>> > + module_put(THIS_MODULE);
>> > +}
>>
>> So is the uevent some standard thing or what? At least grepping
>> INACCESSIBLE for didn't tell me anything.
>
> No, this is not standard. We didn't find any appropriate standard
> message for this case, so we created a new one. Also, we didn't want
> to interfere with components that may react to standard messages.
>
> This is disabled by default and the idea is to change this in the
> future to allow the driver to remove and rescan the device
> automatically. So we will have 3 options in the module parameter: do
> nothing; auto-rescan or; send uevent.
Ok, I assume you will explain this in the commit log.
In my opinion the driver should not be sending custom events like this
and instead pci_stop_and_remove_bus_device() should do it, but maybe
that's just me?
--
Kalle Valo
