Hello,
> + * ATS does not work on rk3588 when running in EP mode.
Would it be OK if we started to style "rk3588" as "RK3588", unless the
lower-case is preferred? I had a look at Rockchip's own datasheet, and the
product code names seem to be styled with upper-case. That said, I am not
a Rockchip expert. Just curious for the sake of consistency.
> +static void rockchip_pcie_ep_hide_broken_ats_cap_rk3588(struct dw_pcie_ep *ep)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct device *dev = pci->dev;
> + unsigned int spcie_cap_offset, next_cap_offset;
> + u32 spcie_cap_header, next_cap_header;
> +
> + /* only hide the ATS cap for rk3588 running in EP mode */
> + if (!of_device_is_compatible(dev->of_node, "rockchip,rk3588-pcie-ep"))
> + return;
> +
> + spcie_cap_offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_SECPCI);
> + if (!spcie_cap_offset)
> + return;
> +
> + spcie_cap_header = dw_pcie_readl_dbi(pci, spcie_cap_offset);
> + next_cap_offset = PCI_EXT_CAP_NEXT(spcie_cap_header);
> +
> + next_cap_header = dw_pcie_readl_dbi(pci, next_cap_offset);
> + if (PCI_EXT_CAP_ID(next_cap_header) != PCI_EXT_CAP_ID_ATS)
> + return;
> +
> + /* clear next ptr */
> + spcie_cap_header &= ~GENMASK(31, 20);
> +
> + /* set next ptr to next ptr of ATS_CAP */
> + spcie_cap_header |= next_cap_header & GENMASK(31, 20);
> +
> + dw_pcie_dbi_ro_wr_en(pci);
> + dw_pcie_writel_dbi(pci, spcie_cap_offset, spcie_cap_header);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +}
To keep things consistent, it would be nice to capitalise sentences in code
comments, and end them with a full-stop where appropriate (e.g., longer
sentences, etc.). That said, this is something I can after applying, to
save you the hassle of sending another versions.
Thank you!
Krzysztof
