Re: [PATCH 2/2] PCI: dw-rockchip: hide broken ATS capability

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

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