Re: [PATCH 2/2] PCI: meson: Add system suspend/resume support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] PCI: meson: accidental clearing of W1C bits in Device Status
  register [1]
--

commit f7a5cff83ce20dd936f701817faa8230379ab3ad
Author: Igor Velkov <iav@iav.lv>

PCI: meson: Add system suspend/resume support

This commit adds noirq suspend and resume callbacks for the Amlogic Meson
PCIe controller when attached to a power domain, along with a
.pme_turn_off callback and power state tracking across host init and
deinit.

> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index 627470c83bb5d..5ef7369bb21bf 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c

[ ... ]

> @@ -385,6 +389,8 @@ static int meson_pcie_host_init(struct dw_pcie_rp *pp)
>  		return ret;
>  	}
>  
> +	mp->powered = true;
> +
>  	pp->bridge->ops = &meson_pci_ops;
>  
>  	meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);

[Severity: Medium]
This problem wasn't introduced by this patch, but can
meson_set_max_payload() and meson_set_max_rd_req_size() inadvertently
clear write-1-to-clear status bits in PCI_EXP_DEVSTA [1] when
meson_pcie_host_init() runs on probe and resume?

drivers/pci/controller/dwc/pci-meson.c:meson_set_max_payload() {
    ...
	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
	val &= ~PCI_EXP_DEVCTL_PAYLOAD;
	dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);

	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
	val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
	dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);
}

Both helpers perform 32-bit read-modify-write accesses at offset +
PCI_EXP_DEVCTL, which includes the 16-bit PCI_EXP_DEVSTA register in the
upper half of the word.  Writing back 1s read from PCI_EXP_DEVSTA clears
pending correctable or non-fatal error status bits, and using
dw_pcie_readw_dbi() and dw_pcie_writew_dbi() would avoid touching
PCI_EXP_DEVSTA.

[1] https://sashiko.dev/#/bug/linux-607d4e29-7318-49b1-a7c3-c91a4c4f26a0

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-b4-pci-meson-suspend-v1-0-be3d04a13304@iav.lv?part=2