Re: [PATCH v7 5/5] PCI/AER: Only clear error bits in pcie_clear_device_status()

Shuai Xue
2026-02-06T07:40:52Z

On 2/3/26 3:53 PM, Lukas Wunner wrote:
> On Sat, Jan 24, 2026 at 03:45:57PM +0800, Shuai Xue wrote:
>> +++ b/drivers/pci/pci.c
>> @@ -2246,7 +2246,7 @@ void pcie_clear_device_status(struct pci_dev *dev)
>> u16 sta;
>>
>> pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &sta);
>> - pcie_capability_write_word(dev, PCI_EXP_DEVSTA, sta);
>> + pcie_capability_write_word(dev, PCI_EXP_DEVSTA, sta & PCI_EXP_DEVSTA_ERR);
>> }
>
> I don't think there's any harm to write error bits which are currently 0,
> so I'd just get rid of the pcie_capability_read_word() and directly write
> the error bits.

Good point. I will remove the read step.

>
>> +++ b/include/uapi/linux/pci_regs.h
>> @@ -534,6 +534,7 @@
>> #define PCI_EXP_DEVSTA_NFED 0x0002 /* Non-Fatal Error Detected */
>> #define PCI_EXP_DEVSTA_FED 0x0004 /* Fatal Error Detected */
>> #define PCI_EXP_DEVSTA_URD 0x0008 /* Unsupported Request Detected */
>> +#define PCI_EXP_DEVSTA_ERR 0xf /* Error bits */
>
> There's only one user of PCI_EXP_DEVSTA_ERR and it feels a little
> awkward to define a macro in a uapi header which does not correspond
> to an "official" bit definition but is just there for convenience.
>
> So maybe it's better to simply use the macros for the four bits in
> pcie_clear_device_status()? Might also be slightly clearer.

Agreed, will move PCI_EXP_DEVSTA_ERR to drivers/pci/pci.c.

>
> This patch could be submitted individually instead of being part
> of this series.

Got it. Will send a individual patch.

>
> Thanks,
>
> Lukas

Thanks for valuable comments.
Shuai