Re: [PATCH 1/5] PCI/portdrv: Don't disable pci device during shutdown

["Krzysztof Wilczyński" <kw@linux.com>]

Hi Huacai,

> Use separate remove()/shutdown() callback, and don't disable pci device

It would be "PCI" here in the above sentence and in the subject line.

> during shutdown. This can avoid some poweroff/reboot failures.

> The poweroff/reboot failures can easily reproduce on Loongson platforms.

Could be better as "can easily be reproduced" in the above.

> I think this is not a Loongson-specific problem, instead, is a problem
> related to some specific PCI hosts. On some x86 platforms, radeon/amdgpu
> devices can cause the same problem, and commit faefba95c9e8ca3a523831c2e
> ("drm/amdgpu: just suspend the hw on pci shutdown") can resolve it.

You might want to change the language to be more imperative in here, as
at the moment I am not sure if you actually have a solution to the
problem here, or you think you have one. :)
 
> As Tiezhu said, this occasionally shutdown or reboot failure is due to
> clear PCI_COMMAND_MASTER on the device in do_pci_disable_device().
> 
> drivers/pci/pci.c
> static void do_pci_disable_device(struct pci_dev *dev)
> {
>         u16 pci_command;
> 
>         pci_read_config_word(dev, PCI_COMMAND, &pci_command);
>         if (pci_command & PCI_COMMAND_MASTER) {
>                 pci_command &= ~PCI_COMMAND_MASTER;
>                 pci_write_config_word(dev, PCI_COMMAND, pci_command);
>         }
> 
>         pcibios_disable_device(dev);
> }
> 
> When remove "pci_command &= ~PCI_COMMAND_MASTER;", it can work well when
> shutdown or reboot. This may implies that there are DMA activities on the
> device while shutdown.
> 
> Radeon driver is more difficult than amdgpu due to its confusing symbol
> names, and I have maintained an out-of-tree patch for a long time [1].
> Recently, we found more and more devices can cause the same problem, and
> it is very difficult to modify all problematic drivers as radeon/amdgpu
> does (the .shutdown callback should make sure there is no DMA activity).
> So, I think modify the PCIe port driver is a simple and effective way.
> And as early discussed, kexec can still work after this patch.
> 
> [1] https://github.com/chenhuacai/linux/commit/8da06f9b669831829416a3e9f4d1c57f217a42f0
[...]

The above explanation and entire backstory is very helpful, but it might
be better to include it in the cover letter, and keep the commit message
here concise and only focused on what is being done here and why.

Krzysztof